Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MISC] Move the PR template to a separate directory and improve contents #338

Merged
merged 1 commit into from
Sep 25, 2019

Conversation

jhlegarreta
Copy link
Contributor

Move the PR template to a separate folder and use HTML syntax.

The first action helps in reducing clutter in the root folder, and helps
identifying more clearly the GitHub-related files.

The HMTL syntax transition using HTML comments allows the PR template
text not to be part of PR message, and saves the user the task of
deleting it while being still clearly visible when opening a PR.

Minor syntax enhancements.

@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Sep 21, 2019

@jhlegarreta
Copy link
Contributor Author

The circleci: linkchecker CI reports two

Result     Error: TooManyRedirects: Exceeded 10 redirects.

errors, but none of the two seems to display the URLs contained in the PR template.

Any guess?

@sappelhoff
Copy link
Member

Looks interesting to me, thanks for the submission. +1 definitely for moving the template into a GitHub folder for clarity in the repository root.

Regarding using an HTML comment, I am undecided: It seems a bit cluttery and it'd distract me from writing my issue (I like to delete the template after reading). But I think that's too much a personal opinion to factor in here ... I'd be fine adding it if popular opinion is in favor.

@franklin-feingold @effigies what are your opinions?

RE: Circle CI:

Any guess?

The HED website seems to be down. This is unrelated to your PR, I think we can ignore it for now and it'll soon be fixed by the responsible people (else, we'll deal with it soon)

@effigies
Copy link
Collaborator

I wouldn't use HTML, because the main way people will see this is through the text editor, not the preview mode. It should be easily readable without further action.

On the whole, I think the current strategy is fine. If you want to create a template with sections that people use, then HTML comments are useful for describing what's expected and letting people choose whether or not to delete them. A big block of text that asks submitters to delete itself seems to work pretty well at getting people to do what we want.

@jhlegarreta
Copy link
Contributor Author

Thanks for the comments !

Shall I then close this PR and open anew one just moving the file to a .githubfolder?

@effigies
Copy link
Collaborator

That'd be fine, though feel free to do it in this PR, to keep the conversation in one place.

Move the PR template to a separate folder to reduce clutter in the root
folder, and help identifying more clearly the GitHub-related files.

Cross-ref explicitly the `CONTRIBUTING` document.

Minor syntax enhancements.
@jhlegarreta jhlegarreta changed the title [MISC] Move the PR template to a separate folder and use HTML syntax [MISC] Move the PR template to a separate folder and improve contents Sep 24, 2019
@jhlegarreta
Copy link
Contributor Author

Done. Please have a look at the new revision dfeb98e . Thanks.

@franklin-feingold
Copy link
Collaborator

franklin-feingold commented Sep 25, 2019

thank you @jhlegarreta for putting this together!

+1 for not having the HTML, mostly on what @effigies described.

I like moving the template away from the Root!

overall LGTM

As @sappelhoff said we can ignore the linkchecker because we know it is from HED not rendering right now, not our system. Side point: If they are down for extended time, can consider perhaps finding a way to skip them in validation so it doesn't appear we are failing our CI

@sappelhoff sappelhoff merged commit 7404e0d into bids-standard:master Sep 25, 2019
@jhlegarreta jhlegarreta deleted the ImprovePRTemplate branch September 25, 2019 19:28
@sappelhoff sappelhoff changed the title [MISC] Move the PR template to a separate folder and improve contents [MISC] Move the PR template to a separate directory and improve contents Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants